Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: [Firebase message][Android] issue open old notify #4203

Closed
wants to merge 5 commits into from
Closed

fix: [Firebase message][Android] issue open old notify #4203

wants to merge 5 commits into from

Conversation

vannt1991
Copy link
Contributor

@vannt1991 vannt1991 commented Sep 3, 2020 •
edited by mikehardy
Loading

Fix issue open old notification

Related issues

Fixes #4052

Release Summary

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

Think react-native-firebase is great? Please consider supporting the project with any of the below:

@CLAassistant
Copy link

CLAassistant commented Sep 3, 2020 •
edited
Loading

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ vannt1991
❌ VanNT

VanNT seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@vercel
Copy link

vercel bot commented Sep 3, 2020 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/8g0aidowf
✅ Preview: https://react-native-firebase-git-fork-vannt1991-master.invertase.vercel.app

@vannt1991 vannt1991 mentioned this pull request Sep 3, 2020
Closed
@vannt1991 vannt1991 changed the title [Firebase message] - Fix issue open old notify [Firebase message] - [Android] Fix issue open old notify Sep 3, 2020
@vannt1991 vannt1991 changed the title [Firebase message] - [Android] Fix issue open old notify fix: [Firebase message][Android] issue open old notify Sep 3, 2020
pavel-corsaghin
pavel-corsaghin previously approved these changes Sep 3, 2020
View reviewed changes
@mikehardy
Copy link
Collaborator

Awesome! I will review as soon as I can

We must have the CLA signed before we can merge it

Anyone can use these auto generated patches from the PR for testing - I would love independent confirmation it works for someone else? https://github.com/invertase/react-native-firebase/suites/1137916996/artifacts/16295093

@mikehardy
Copy link
Collaborator

The patches are intended to drop right in the patches directory of your app, and are in patch-package format, forgot to mention. The idea is it is very easy to get testers - this is a new features in our repo for PRs so I have not documented it yet sorry

@mikehardy mikehardy requested a review from Salakar September 4, 2020 14:28
@mikehardy mikehardy added Workflow: Needs Review Pending feedback or review from a maintainer. Workflow: Waiting for User Response Blocked waiting for user response. labels Sep 4, 2020
@mikehardy
Copy link
Collaborator

@vannt1991 this looks much closer! I need to review it really closely now with the idea I can merge it, however we still need the CLA signed or I can't merge no matter what 🙏

@vannt1991
Copy link
Contributor Author

@mikehardy
i have changed to this request, pls help me approve it, many thanks 👍
#4317

@mikehardy mikehardy removed Workflow: Needs Review Pending feedback or review from a maintainer. Workflow: Waiting for User Response Blocked waiting for user response. labels Sep 28, 2020
@mikehardy mikehardy closed this Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikehardy mikehardy mikehardy left review comments

@pavel-corsaghin pavel-corsaghin pavel-corsaghin left review comments

@Salakar Salakar Awaiting requested review from Salakar

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Android: getInitialNotification/onNotificationOpenedApp not triggered for old notifications if app restarts
4 participants
@vannt1991 @CLAassistant @mikehardy @pavel-corsaghin